Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add module : scene-referred blurs #9521

Merged
merged 4 commits into from Jul 23, 2021

Conversation

aurelienpierre
Copy link
Member

@aurelienpierre aurelienpierre commented Jul 13, 2021

Add support for physically-accurate motion, lens and gaussian blurs in linear RGB.

This is slow code for now, doing convolution products in spatial space, CPU and OpenCL variants provided. A faster convolution approach has been tested (commented out in the code), using FFT, but crashes with no explanation, and I don't have time to investigate that now. That shall be investigated later, the current code is simple and functional.

Warning : to test, in lens blur mode, to get proper stars, you need to ensure concavity < blades - 2. If that condition is not met, you get degenerated shape (which can have merits too, but is not what you expect).

Example pics by @johnny-bit :
Screenshot_20210713_142507
Screenshot_20210713_142624
Screenshot_20210713_142530
Screenshot_20210713_142905

Original:
Screenshot_20210713_143322

Add support for motion, lens and gaussian blurs
in linear RGB.
@aurelienpierre aurelienpierre added this to the 3.8 milestone Jul 13, 2021
@aurelienpierre aurelienpierre added the scope: image processing correcting pixels label Jul 13, 2021
@johnny-bit johnny-bit added the documentation-pending a documentation work is required label Jul 13, 2021
@ralfbrown
Copy link
Collaborator

Any chance of running this in reverse to undo motion blur? You have what looks like a nice interface to set up the point spread function used by most (all?) deconvolution algorithms.

@aurelienpierre
Copy link
Member Author

Any chance of running this in reverse to undo motion blur? You have what looks like a nice interface to set up the point spread function used by most (all?) deconvolution algorithms.

Wow, definitely not. Deconvolution is a whole different game than this, and the methods that actually work ok are super slow and a lot more work.

@johnny-bit
Copy link
Member

Howeverrrr if one were to optimize it (like diffuse got optimized ;) ) this are the words of the Aurthor:

mmmh, optimization means using FFT direct and inverse transforms
then multiplying only the real parts of both FFT-transformed arrays
it's not just loop reordering here
he whole logic changes
but that will be for another time [...]

Just a note for optimizers ;)

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments.

You also need to update po/POTFILE.in

And as for diffuse you need to add this new module into some of the module group presets.
See src/libs/modulegroups.c where presets are defined.

You need to at least add it in "module: all" preset and maybe some others.

Can you also do that for diffuse ?

TIA.

src/iop/blurs.c Outdated Show resolved Hide resolved
src/iop/blurs.c Outdated Show resolved Hide resolved
src/iop/blurs.c Outdated Show resolved Hide resolved
src/iop/blurs.c Outdated Show resolved Hide resolved
@aurelienpierre
Copy link
Member Author

@TurboGit I will deal with the module presets in a separate PR

@@ -186,6 +186,7 @@ src/iop/basicadj.c
src/iop/bilat.c
src/iop/bilateral.cc
src/iop/bloom.c
src/iop/blurs.c
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do it just after merging, but for reference you always have two entries to add, one for the module itself and one for the generated introspection code.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good now, thanks !

@TurboGit TurboGit merged commit 923ebfa into darktable-org:master Jul 23, 2021
@s7habo
Copy link

s7habo commented Jul 25, 2021

@aurelienpierre I just tested it and it works great, but I wonder why the radius is limited to 128 px?

For such a scene....

original:

_DSC7694_02b

blur:

_DSC7694_02a

...I had to use three instances.

But in itself, with new blurs module you can do cool things :)
Thank you!

@aurelienpierre
Copy link
Member Author

A radius of 128 px means a full blur of 256 px, which means at each pixel, you need to compute 256×256 multiplications and additions. Given that the module is not fully optimized right now, that's already insanely slow.

@elstoc elstoc added documentation-complete needed documentation is merged in dtdocs and removed documentation-pending a documentation work is required labels Nov 11, 2021
@aurelienpierre aurelienpierre deleted the physical-blurs branch December 12, 2022 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-complete needed documentation is merged in dtdocs scope: image processing correcting pixels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants